DOC: update the DatetimeIndex.tz_convert(tz) docstring #20096

hammadmashkoor · 2018-03-10T06:02:55Z

Checklist for the pandas documentation sprint (ignore this if you are doing
an unrelated PR):

PR title is "DOC: update the docstring"
The validation script passes: scripts/validate_docstrings.py <your-function-or-method>
The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
The html version looks good: python doc/make.py --single <your-function-or-method>
It has been proofread on language by another sprint participant

Please include the output of the validation script below between the "```" ticks:

(pandas_dev) root@kalih:~/pythonpanda/pandas# python -m flake8 ./pandas/core/indexes/datetimes.py
(pandas_dev) root@kalih:~/pythonpanda/pandas# python scripts/validate_docstrings.py pandas.DatetimeIndex.tz_convert

################################################################################
################# Docstring (pandas.DatetimeIndex.tz_convert)  #################
################################################################################

Convert tz-aware DatetimeIndex from one time zone to another.

When using DatetimeIndex providing with timezone this method
converts tz(timezone)-aware DatetimeIndex from one timezone
to another.

Parameters
----------
tz : string, pytz.timezone, dateutil.tz.tzfile or None
    Time zone for time. Corresponding timestamps would be converted
    to time zone of the DatetimeIndex.
    None will remove timezone holding UTC time.

Returns
-------
normalized : DatetimeIndex

Raises
------
TypeError
    If DatetimeIndex is tz-naive.

See Also
--------
tz_localize : Localize tz-naive DatetimeIndex to given time zone,
              or remove timezone from tz-aware DatetimeIndex.

Examples
--------
With the `tz` parameter, we can change the DatetimeIndex
to other time zones:

>>> dti = pd.DatetimeIndex(start='2014-08-01 09:00',
...                       freq='H', periods=3)

>>> dti
DatetimeIndex(['2014-08-01 09:00:00',
               '2014-08-01 10:00:00',
               '2014-08-01 11:00:00'],
                dtype='datetime64[ns]', freq='H')

>>> dti.tz_localize('Europe/Berlin').tz_convert('US/Eastern')
DatetimeIndex(['2014-08-01 03:00:00-04:00',
               '2014-08-01 04:00:00-04:00',
               '2014-08-01 05:00:00-04:00'],
                dtype='datetime64[ns, US/Eastern]', freq='H')

With the `None` parameter, we can remove the timezone:

>>> dti = pd.DatetimeIndex(start='2014-08-01 09:00',freq='H',
...                        periods=3, tz='Europe/Berlin')

>>> dti
DatetimeIndex(['2014-08-01 09:00:00+02:00',
               '2014-08-01 10:00:00+02:00',
               '2014-08-01 11:00:00+02:00'],
                dtype='datetime64[ns, Europe/Berlin]', freq='H')

>>> dti.tz_convert(None)
DatetimeIndex(['2014-08-01 07:00:00',
               '2014-08-01 08:00:00',
               '2014-08-01 09:00:00'],
                dtype='datetime64[ns]', freq='H')

################################################################################
################################## Validation ##################################
################################################################################

Docstring for "pandas.DatetimeIndex.tz_convert" correct. :)

If the validation script still gives errors, but you think there is a good reason
to deviate in this case (and there are certainly such cases), please state this
explicitly.

…eIndexTzConvert

IHackPy

try to make summary more clear
tz --> timezone

…eIndextzconvertfunc

codecov · 2018-03-10T09:39:25Z

Codecov Report

❗ No coverage uploaded for pull request base (master@c748de0). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #20096   +/-   ##
=========================================
  Coverage          ?   91.72%           
=========================================
  Files             ?      150           
  Lines             ?    49156           
  Branches          ?        0           
=========================================
  Hits              ?    45090           
  Misses            ?     4066           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.11% <100%> (?)`
#single	`41.85% <100%> (?)`

Impacted Files	Coverage Δ
pandas/core/indexes/datetimes.py	`95.64% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c748de0...ff00665. Read the comment docs.

jorisvandenbossche

Thanks for the PR! Added some comments

jorisvandenbossche · 2018-03-10T09:09:40Z

pandas/core/indexes/datetimes.py

-        Convert tz-aware DatetimeIndex from one time zone to another (using
-        pytz/dateutil)
+        Convert tz-aware DatetimeIndex from one
+        time zone to another.


Can you put this on one line? (I think it should fit within 80 chars)

jorisvandenbossche · 2018-03-10T10:27:07Z

pandas/core/indexes/datetimes.py

@@ -1904,24 +1904,47 @@ def delete(self, loc):

    def tz_convert(self, tz):
        """
-        Convert tz-aware DatetimeIndex from one time zone to another (using
-        pytz/dateutil)
+        Convert tz(timezone)-aware DatetimeIndex using pytz/dateutil.


I would just use 'timezone' instead of both the abbreviation and full. You can maybe put the abbreviation and explanation in the extended summary

I think it is also fine to leave the "using pytz/dateutil." for the extended summary, as this is already quite a detail.

jorisvandenbossche · 2018-03-10T10:27:59Z

pandas/core/indexes/datetimes.py


        Parameters
        ----------
        tz : string, pytz.timezone, dateutil.tz.tzfile or None
-            Time zone for time. Corresponding timestamps would be converted to
-            time zone of the TimeSeries.
+            Time zone for time.Corresponding timestamps would be converted


You seemed to have removed a space between the two words, which I suppose was a mistake.

jorisvandenbossche · 2018-03-10T10:28:26Z

pandas/core/indexes/datetimes.py

            None will remove timezone holding UTC time.

        Returns
        -------
        normalized : DatetimeIndex

        Raises
-        ------
+        -------


This change is not needed (the line under the title can be exactly as long as the title itself)

jorisvandenbossche · 2018-03-10T10:29:03Z

pandas/core/indexes/datetimes.py

+        0   2018-02-28 19:00:00-05:00
+        1   2018-03-01 19:00:00-05:00
+        2   2018-03-02 19:00:00-05:00
+        dtype: datetime64[ns, US/Eastern]


Can you also add an example that uses tz_convert(None) ?
It's explained at the end of this section: https://pandas.pydata.org/pandas-docs/stable/timeseries.html#working-with-time-zones

jorisvandenbossche

Did you run the validate_docstrings script again?

jorisvandenbossche · 2018-03-10T11:47:14Z

pandas/core/indexes/datetimes.py

+        Examples
+        --------
+        >>> didx = pd.DatetimeIndex
+        (start='2014-08-01 09:00', freq='H', periods=10, tz='Europe/Berlin')


periods=3 is long enough I think to illustrate the example, and will give a shorter output

jorisvandenbossche · 2018-03-10T11:47:44Z

pandas/core/indexes/datetimes.py

+        '2014-08-01 11:00:00-04:00', '2014-08-01 12:00:00-04:00'],
+        dtype='datetime64[ns, US/Eastern]', freq='H')
+
+        >>> didx.tz_convert(None)


Can you put here a small sentence explaining the example?

@jorisvandenbossche didn't realize we actually allow None here.

jreback · 2018-03-10T13:08:05Z

pandas/core/indexes/datetimes.py

+
+        When using DatetimeIndex providing with timezone this method
+        converts tz(timezone)-aware DatetimeIndex from one timezone
+        to another using pytz/dateutil.


you can remove the pytz/dateutil part here

let's coordinate the text in this doc-string with #20050 which is sister operation.

I have removed the pytz/dateutil part

jreback · 2018-03-10T13:10:57Z

pandas/core/indexes/datetimes.py

+        '2014-08-01 11:00:00-04:00', '2014-08-01 12:00:00-04:00'],
+        dtype='datetime64[ns, US/Eastern]', freq='H')
+
+        >>> didx.tz_convert(None)


@jorisvandenbossche didn't realize we actually allow None here.

jreback · 2018-03-10T14:26:36Z

pandas/core/indexes/datetimes.py

-        pytz/dateutil)
+        Convert tz-aware DatetimeIndex from one time zone to another.
+
+        When using DatetimeIndex providing with timezone this method


This is a cumbersome sentence and is pretty duplicative of the above, I would remove it.

I regret the auto typing mistake, I wanted to convey that i will surely be doing it as per recommended by you.
Please pardon my Indian English.

@hammadmashkoor no problem. thanks for the PR!

I don't think this extended summary is needed at all as its duplicated of the Summary.

jreback · 2018-03-10T14:27:07Z

pandas/core/indexes/datetimes.py

-            Time zone for time. Corresponding timestamps would be converted to
-            time zone of the TimeSeries.
+            Time zone for time. Corresponding timestamps would be converted
+            to time zone of the TimeSeries.


TimeSeries -> DatetimeIndex

jreback · 2018-03-10T14:27:19Z

pandas/core/indexes/datetimes.py

+        See Also
+        --------
+        tz_localize : Localize tz-naive DatetimeIndex to given time zone
+            (using pytz/dateutil), or remove timezone from tz-aware


remove using pytz/dateutil

jreback · 2018-03-10T14:27:31Z

pandas/core/indexes/datetimes.py

+                       '2014-08-01 11:00:00+02:00'],
+                        dtype='datetime64[ns, Europe/Berlin]', freq='H')
+
+        With the `tz` parameter, we can change DatetimeIndex


jreback · 2018-03-10T14:27:42Z

pandas/core/indexes/datetimes.py

+                       '2014-08-01 05:00:00-04:00'],
+                        dtype='datetime64[ns, US/Eastern]', freq='H')
+
+        With the `None` parameter, we can remove timezone:


remove the timezone

jreback · 2018-03-10T15:22:13Z

pandas/core/indexes/datetimes.py

+
+        Examples
+        --------
+        >>> data=pd.DatetimeIndex(start='2014-08-01 09:00',


call this dti . (rather than data)

and spaces around the equals

jreback · 2018-03-10T16:06:10Z

pandas/core/indexes/datetimes.py

+        With the `tz` parameter, we can change the DatetimeIndex
+        to other time zones:
+
+        >>> dti.tz_convert('US/Eastern')


so THIS doesn't work (well it works but doesn't do what you want).

Create 2 example dti's here, the first with NO timezone (like you had originally), and .tz_localize('Europe/Berlin')
the second with tz='Europe/Berlin' then use .tz_localize(None), this shows removing it.

In case of tz_convert(tz), do we have to use tz_localize() ?

jreback · 2018-03-10T16:59:26Z

pandas/core/indexes/datetimes.py

+                       '2014-08-01 11:00:00'],
+                        dtype='datetime64[ns]', freq='H')
+
+        >>> dti.tz_localize('Europe/Berlin').tz_convert('US/Eastern')


I wouldn't use tz_convert here, this is about the tz_localize doc-string

jreback · 2018-03-10T16:59:36Z

pandas/core/indexes/datetimes.py

+                       '2014-08-01 11:00:00+02:00'],
+                        dtype='datetime64[ns, Europe/Berlin]', freq='H')
+
+        >>> dti.tz_convert(None)


yes this is correct

…eIndextzconvertfunc

hammadmashkoor · 2018-03-13T09:06:21Z

made the required changes, please review @jreback

jreback

examples look good. some small grammer corrections.

jreback · 2018-03-13T10:01:15Z

pandas/core/indexes/datetimes.py

-        pytz/dateutil)
+        Convert tz-aware DatetimeIndex from one time zone to another.
+
+        When using DatetimeIndex providing with timezone this method


I don't think this extended summary is needed at all as its duplicated of the Summary.

jreback · 2018-03-13T10:01:31Z

pandas/core/indexes/datetimes.py

-            Time zone for time. Corresponding timestamps would be converted to
-            time zone of the TimeSeries.
+            Time zone for time. Corresponding timestamps would be converted
+            to time zone of the DatetimeIndex.


to time zone of the DatetimeIndex -> to this time zone.

jreback · 2018-03-13T10:02:41Z

pandas/core/indexes/datetimes.py

+
+        See Also
+        --------
+        tz_localize : Localize tz-naive DatetimeIndex to given time zone,


to a given time zone

or remove timezone from a tz-aware DatetimeIndex

…eIndextzconvertfunc

Formatting on See Also Added tz PEP8 on example Switched to tz_convert

TomAugspurger · 2018-03-13T12:12:29Z

Made a few changes. Biggest one being changing the first example to demonstrate tz_convert instead of tz_localize.

TomAugspurger · 2018-03-13T12:45:31Z

Thanks @hammadmashkoor !

root and others added 3 commits March 10, 2018 02:23

Added Docstring for DatetimeIndex.tz_convert(tz)

b062d66

Merge remote-tracking branch 'upstream/master' into docstring_Datetim…

82c6546

…eIndexTzConvert

Doc:added for TimedateIndex.tz_convert(tz)

d16d9ef

IHackPy reviewed Mar 10, 2018

View reviewed changes

hammadmashkoor added 2 commits March 10, 2018 15:06

Merge remote-tracking branch 'upstream/master' into docstring_Datetim…

2179e2d

…eIndextzconvertfunc

Update Docstring

74ba82a

jorisvandenbossche reviewed Mar 10, 2018

View reviewed changes

Updated on recent comments

4f7a570

jorisvandenbossche reviewed Mar 10, 2018

View reviewed changes

jreback requested changes Mar 10, 2018

View reviewed changes

jreback added Docs Timezones Timezone data dtype labels Mar 10, 2018

Updated under comments

7a003de

jreback requested changes Mar 10, 2018

View reviewed changes

Updated under recommendations

fac2544

jreback requested changes Mar 10, 2018

View reviewed changes

Updated under recommendations

236e00b

jreback requested changes Mar 10, 2018

View reviewed changes

Updated under recommendations

1b56342

jreback requested changes Mar 10, 2018

View reviewed changes

hammadmashkoor added 2 commits March 10, 2018 22:40

Updated under recommendations

f8e4024

Merge remote-tracking branch 'upstream/master' into docstring_Datetim…

c136136

…eIndextzconvertfunc

jreback requested changes Mar 13, 2018

View reviewed changes

hammadmashkoor and others added 3 commits March 13, 2018 15:52

Merge remote-tracking branch 'upstream/master' into docstring_Datetim…

3d9b5b5

…eIndextzconvertfunc

Updated

1f0adfc

Formatting on tz

ff00665

Formatting on See Also Added tz PEP8 on example Switched to tz_convert

TomAugspurger merged commit 1837929 into pandas-dev:master Mar 13, 2018

Uh oh!

DOC: update the DatetimeIndex.tz_convert(tz) docstring #20096

DOC: update the DatetimeIndex.tz_convert(tz) docstring #20096

Uh oh!

Conversation

hammadmashkoor commented Mar 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IHackPy left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hammadmashkoor commented Mar 13, 2018

hammadmashkoor commented Mar 10, 2018 •

edited

Loading

codecov bot commented Mar 10, 2018 •

edited

Loading